Skip to content

update yamls to adapt to oops/pull/2434 #986

Merged
travissluka merged 4 commits intodevelopfrom
feature/io_for_obsbiasincrement
Dec 13, 2023
Merged

update yamls to adapt to oops/pull/2434 #986
travissluka merged 4 commits intodevelopfrom
feature/io_for_obsbiasincrement

Conversation

@travissluka
Copy link
Contributor

Description

update yamls for pending oops PR
FYI @guillaumevernieres @ShastriPaturi , you'll probably have to make a similar change in your workflow

dependencies

build-group=https://github.com/JCSDA-internal/oops/pull/2434

@travissluka travissluka self-assigned this Dec 8, 2023
@travissluka travissluka added the waiting for another PR waiting on another pull request to be merged first label Dec 8, 2023
Copy link
Contributor

@guillaumevernieres guillaumevernieres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

@ShastriPaturi ShastriPaturi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving it out of turn.

Copy link

@leungtszyan leungtszyan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see there is at least one other file (test/testinput/3dvar_single_ob.yaml) that hasn't implemented the change. Please check through all the test YAML files again.

@leungtszyan
Copy link

Thanks for pushing the change. One more thing: the variational.iterations[n].online diagnostics.increment section also needs to be changed in a similar way (example), and I think there are multiple files that have this section in the YAML configuration.

@travissluka
Copy link
Contributor Author

Thanks for pushing the change. One more thing: the variational.iterations[n].online diagnostics.increment section also needs to be changed in a similar way (example), and I think there are multiple files that have this section in the YAML configuration.

the ctests pass, the yaml files for all of the active test have already been updated. Anything left is not a test being exercised. Thank you for taking a look though.

@leungtszyan leungtszyan self-requested a review December 11, 2023 17:21
Copy link

@leungtszyan leungtszyan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pushing the change. One more thing: the variational.iterations[n].online diagnostics.increment section also needs to be changed in a similar way (example), and I think there are multiple files that have this section in the YAML configuration.

Sorry I meant to refer to this example instead of the one hyperlinked above. Anyway, thanks for letting me know that this is not an active test. I can indeed see this in the CMakeLists.txt file. However, I would recommend that you update that YAML file as well, so that when the time comes for the test to be activated, the build could proceed smoothly. That being said, I am approving this PR in its current form, as it's not expected to break the existing tests as you said.

@symoore90 symoore90 added the SOCA Sea-ice, Ocean, and Coupled Assimilation label Dec 12, 2023
@shlyaeva
Copy link
Collaborator

oops PR was merged, this is ready to merge now.

@travissluka travissluka merged commit c6641b8 into develop Dec 13, 2023
@travissluka travissluka deleted the feature/io_for_obsbiasincrement branch December 13, 2023 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

SOCA Sea-ice, Ocean, and Coupled Assimilation waiting for another PR waiting on another pull request to be merged first

Projects

None yet

Development

Successfully merging this pull request may close these issues.

adapt to oops/pull/2434

7 participants